Skip to content

Server-side backend: a native, JVM-free runtime for Codename One handlers - #5741

Open
shai-almog wants to merge 176 commits into
masterfrom
backend-throughput
Open

Server-side backend: a native, JVM-free runtime for Codename One handlers#5741
shai-almog wants to merge 176 commits into
masterfrom
backend-throughput

Conversation

@shai-almog

Copy link
Copy Markdown
Collaborator

Adds a server-side runtime that runs a Codename One handler through the ParparVM
pipeline: Java or Kotlin translated to C and compiled into one static native
executable with no JVM under it. About 8 MB, a few milliseconds to first
connection, about 3 MB idle.

What this is for, and what it is not

It does not replace Spring Boot, Jakarta EE, Quarkus or Micronaut, and it is not
trying to. Those carry a container, an ORM, a security stack and twenty years of
operations; none of that is here or planned.

It targets the region where the JVM's assumptions stop paying: cold starts
charged per invocation, baseline memory charged for an instance's life, sidecars,
edge locations, short-lived processes. That is where Java is thin and Go and
Node dominate, and where a Java shop ends up carrying a second language and a
second copy of every model that crosses the boundary. Either as a piece of a
larger deployment or as the whole server for a small project.

The vertical integration is the other half: one @RestClient interface generates
the app's asynchronous client and the backend's synchronous half plus its
dispatcher, so a contract change is a compile error rather than a response the
app fails to parse in the field.

Where it stands against Go

vm/backend/benchmarks holds the harness. Two pinned cores, 64 connections,
interleaved with rotating arm order, against fasthttp:

CN1 fasthttp
/plaintext throughput 668k rps (n=14 paired) 631k
/plaintext p50 / p99 70 us / 183 us 84 us / 980 us
/json throughput 625k rps (n=16 paired) 615k
/json p50 87 us 97 us
/json RSS 24.7 MB 12.0 MB

The /json figures are the generated-DTO path answering off a pooled response.
A handler that returns a LinkedHashMap per request is about 0.58x, which the
benchmark keeps as its default because that is the honest cost of that shape.

Notable changes outside vm/backend

  • cn1_globals.m gains cn1SatbTrim. The SATB write-barrier log and its staging
    buffer only ever doubled and were never given back, so a process that saw one
    busy period kept the peak for life -- 8 MB of a 12 MB plaintext process was an
    empty buffer. Trimmed in the sweep against the recent high-water mark. This
    reaches every Codename One target, not just the backend.
  • maven/pom.xml builds maven/backend, which was in no <modules> block, so
    nothing built the artifact BackendPackageMojo resolves at run time.
  • Two goals, cn1:backend and cn1:backend-package, and the @RestClient
    server-half processor.
  • The archetype and the initializr both generate a backend module, behind
    -Dcodename1.platform=backend so a client-only app pays nothing for it.
  • A developer-guide chapter under a new "Server side" part.

Testing

  • BackendHttpIntegrationTest 21/21, plus the database and JavaSE-runtime suites.
  • GC suites: GcHeapIntegrity, GcOverflowSpiral, GcUncooperativeThread,
    LargeArrayGc, BibopPageFloor.
  • GcSteadyState's 768 MB ceiling scenario fails on the dev machine and fails
    identically with the SATB change stashed (895.8s against 913.7s, same timeout,
    same scenario), so it is the known local failure rather than a regression. It
    is @Tag("benchmark") and runs in the benchmark job.
  • Guide gates: vale 0 issues, asciidoctor clean at --failure-level WARN,
    structure, cross-references, snippets, links, paragraph capitalization.
  • SpotBugs on codenameone-maven-plugin: 0 findings. Copyright, control
    characters and cast-semantics gates clean over the branch.
  • The archetype was installed, a project generated from it, and the generated
    backend module compiled against codenameone-backend.

PMD and Checkstyle were not run locally; CI is the first run for those.

shai-almog and others added 30 commits September 1, 2026 21:43
The clean (non-Objective-C) target could translate a Java main() and run it, but
not much more: main(String[]) was handed JAVA_NULL, so a translated program could
not read its own command line, and there was no way to read the environment, open
a file or read stdin. Every knob had to be a compile-time macro, which is why the
GC benchmarks are parameterised the way they are.

  - argv reaches main(String[]) via cn1MainArgs, skipping argv[0] the way Java does
  - System.getenv(String)
  - java.io.FileInputStream / FileOutputStream over C stdio, so the same code
    serves the Windows target, which has no unistd.h
  - java.io.StandardInputStream behind System.in. Not a FileInputStream: stdin is
    not seekable, so skip and available cannot be answered by seeking

Separately, CHECKCAST. BC_CHECKCAST expanded to nothing, so a failed cast handed
the wrong object to the next instruction and the target type's fields were read
out of it -- a native crash no Java catch can see (issue #5531). Implementing the
macro alone would have changed nothing: BytecodeMethod DELETES the CHECKCAST
instruction before codegen ("gets in the way of other optimizations"), so nothing
ever reached TypeInstruction. Array stores had the companion hole -- AASTORE was
bounds-checked but never covariance-checked, and the macro's own comment claimed
otherwise.

Both are now enforced under -Dcn1.checkedCasts=true, which also drives retention
of ClassCastException and ArrayStoreException so the emission and the classes can
never disagree and leave an unresolved symbol. Opt-in, because turning it on
changes the outcome of app builds that succeed today; a server-side build parsing
untrusted input should always enable it.

Verified against vm/tests: 80 tests, no regressions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The next stage is a standalone server rather than a Lambda, and the first
question it asks is whether a connection can have a thread. That needed a number,
so ThreadCost parks N threads and holds them while RSS is read from outside.

Measured with 512 parked threads:

    musl/arm64 (the deployment target)   243 KB/thread
    macOS/arm64                          118 KB/thread

Attribution on Linux, by ablation:

    callStack arrays  (1024 -> 128)      -50 KB
    pendingHeapAllocations (4096 -> 256) -27 KB
    try blocks (500 -> 32)               -15 KB
    shadow stack (16536 -> 2048)           0 KB
    thread stack (16MB -> 256KB)           0 KB

Two of those are worth recording because they are the opposite of what the
macOS numbers suggested. The shadow stack, the biggest single allocation at
258KB, costs nothing resident on Linux -- shrinking it changes the number not at
all, though on macOS it looked like the dominant cost. And the pinned 16MB thread
stack is free: it is reserved, never committed.

The five sizes are now #ifndef-guarded so an A/B can override them with -D. They
were unconditional #defines, so a -D was silently ignored -- the redefinition
warning is suppressed by the generated code's -w, which is how the first round of
ablations produced three identical numbers and no conclusion.

The shadow stack is now mapped rather than malloc'd and memset in full. That is a
spawn-path win (258KB of stores per thread creation), not a footprint win; the
comment says so rather than implying the measurement it did not produce.

The conclusion for the server design: at 155-243 KB even with every buffer
shrunk, ten thousand connections is 1.5-2.4GB of threads. A connection cannot have
one. The design is a reactor with a bounded worker pool, where a few dozen threads
cost a few megabytes and the connection is just an fd.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
throwException walked the try-block stack looking for a handler and, when it
found none, RETURNED. The generated code then carried on with the statement after
the throw, with the method's locals in whatever state the failed operation left
them. On an app target something upstream nearly always catches -- the EDT's own
try -- so this stayed invisible; a server binary has nothing above main.

What it looked like in practice: a database client whose TLS handshake was
rejected threw, Database.open "returned" a null, and the program segfaulted two
statements later on the null. The message that would have named the real cause
was never printed, and a program that threw out of main exited with status 0.

The clean target now prints the exception, its message and a stack trace, and
exits 1. Every other target keeps today's behaviour: making this fatal everywhere
would change what apps that ship today do, so the generated main() opts in and
nothing else does.

Two details the fix needed. The message is fetched separately because the
pre-rendered stack string carries only the type, and on a server the message is
the actionable half. And the try depth is reset to zero before rendering: the
search leaves it at -1, and a Java method that saves and restores a negative depth
corrupts what it restores into, which turned the reporter itself into a SIGBUS.

Also here, because the same audit found it: java.lang.System.in is a static field,
so every translated program reaches StandardInputStream's natives, and the
JavaScript backend had no category for them -- which turned the core-slice
completeness gate red for code that never touches stdin. They are marked
unsupported there, as java.io.File already is: a browser has no process stdin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are one-line consequences of the same C rule, found by building the same
program two ways.

ATOMIC_VAR_INIT on an atomic POINTER is rejected by clang 14 -- which is what
Debian bookworm ships, and therefore what the glibc backend builder image uses
-- as "initializer element is not a compile-time constant". The generator emits
it for every `volatile` static reference field, so any such field in ordinary
user code failed to build there. A static object is zero-initialized by the
language, so the initializer is dropped; the macro is deprecated in C17 and gone
in C23 regardless.

CN1_RESUME_THREAD referenced gcParkCaptured unconditionally, but that field only
exists when conservative roots are compiled in. So
-DCN1_DISABLE_CONSERVATIVE_GC_ROOTS -- the A/B arm vm/CLAUDE.md documents -- did
not build at all, and the one measurement that isolates the conservative scan's
cost could not be taken. It is now behind a macro that compiles away with the
field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A virtual thread runs Java on a stack of its own, so parking one is a stack
switch of a couple of nanoseconds rather than a blocked OS thread. Measured
round trip on arm64: 2.1ns.

The runtime is three files -- cn1_virtual_thread.{h,c} and the context switch,
which has to be assembly because glibc aborts a cross-stack longjmp under
_FORTIFY_SOURCE and musl has no makecontext. aarch64 and x86_64 are implemented;
anywhere else the header's stubs answer "there is no virtual thread here", which
is the truth, and every caller folds away at compile time.

The collector had to learn about them, because a virtual thread breaks two of its
assumptions silently:

  - A carrier RUNNING a virtual thread has its stack pointer inside that virtual
    stack, so the [sp, base) bounds test rejected it and skipped every
    conservative root the thread held.
  - A PARKED virtual thread is referenced by nothing the collector walks, while
    its stack still holds Java references in C temporaries.

Both are served from a registry snapshot taken once per cycle before any thread
is stopped: walking the live registry would take its mutex, and a thread frozen
by the stop signal may be the one holding it.

Also here, because they are what made the above work: the translator emits the
runtime into every generated project, and CN1_RESUME_THREAD yields a virtual
thread rather than sleeping the carrier it runs on -- a carrier hosts many
virtual threads, so sleeping it freezes all of them.

Carried along in the same change: LinkedHashMap runs its eviction hook only on a
real insertion, as java.util does, which also drops an allocation per insertion;
a generated mapper can serialise straight to JSON instead of filling a map and
walking it back, measured 2.05x/1.51x/2.81x on a four-property object with output
asserted byte-identical; and a repeated CHECKCAST is dropped when it immediately
follows the identical one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CN1_RESUME_THREAD waited out a collection with usleep(1000). Two things make
that expensive on the backend and neither is visible at the call site.

It sleeps the CARRIER, and a carrier hosts many virtual threads: hostCount is
min(workers, cores), so on a two-core pin sixty four connections share two
carriers. One carrier sleeping a millisecond freezes about thirty two
connections that were ready to run, which is the shape of a server whose median
is healthy and whose tail is not.

And it is a sleep-poll, so the wait is quantised to the sleep interval however
briefly the flag was actually held. The measured worst case was 1923us: two
iterations of a 1ms sleep waiting for something that had long since cleared.

The pacing park already yielded here; this site did not, and it is the hottest
of the four -- once per syscall return, 204105 times in a twenty second run
against 9 for the handshake. Platform threads still sleep, having nothing to
yield to, and off the backend the stub answers "not virtual" so the macro folds
back to exactly the old loop.

This shortens the wait; it does not remove it. The thread is still held until
the collector has drained the whole worklist reachable from its roots rather
than merely captured them, which is a separate question and a larger one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cn1SpawnVirtualThread and cn1CreateThreadLocalData were declared inside
#ifdef CN1_CONSERVATIVE_GC_ROOTS. Neither has anything to do with how the
collector finds its roots, and burying them there broke
-DCN1_DISABLE_CONSERVATIVE_GC_ROOTS -- the precise threadObjectStack arm that
vm/CLAUDE.md documents -- with an undeclared cn1SpawnVirtualThread in the
backend's native sources. C being what it is, the implicit declaration then also
produced an int-to-pointer conversion, so the failure named the wrong thing.

Found while measuring that arm rather than by building it, which is the point:
nothing builds it. The default build is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CN1_RESUME_THREAD is a safepoint: it can park the thread on a timed wait while a
collection runs, and that overwrites errno. Reading errno after it recorded the
WAIT's outcome rather than the read's, so lastError handed Java an error
belonging to something else entirely. Captured at the syscall instead.

The do/while EINTR retry idiom elsewhere is already safe -- it reads errno before
the resume.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mark phase signals every thread and spins until it answers, so it can scan
the thread's native stack conservatively. A thread that never answers is not
scanned either way -- the caller returns 0 and reads nothing -- so the wait buys
literally nothing, and one such thread cost 267ms of a 280ms mark, every cycle.

Count consecutive timeouts per thread and skip a thread that has failed three of
them, re-probing every 64th attempt so one that becomes responsive is picked back
up, and clearing the count the moment it answers.

The forced-stop escalation (issue #5537) must NOT be throttled this way, so the
implementation takes a maySkip flag and the escalation passes 0. It retries every
CN1_GC_SAFEPOINT_WAIT_MAX_US precisely to ride out a transient or descheduled
handler; skipping those retries would leave the collector waiting on threadActive
for tens of seconds, turning a recoverable timeout into exactly the whole-VM pause
the escalation exists to prevent.

Measured on the server workload: stackMs 269 -> 0.20.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sembly

Two halves of one bug. Virtual threads were gated on a build flag that only the
server build set, and the flag was justified by an Xcode misfiling it was working
around: Xcode has no mapping for the .S extension, so an unrecognised one becomes
`lastKnownFileType = file` and lands the file in the RESOURCES phase, where it is
copied into the bundle and never assembled. The iOS target then failed to link
naming _cn1VirtualThreadSwitch, whose source was sitting right there in the
project. Gating the feature off made the misfiled resource inert, so the phone
target linked and the misfiling stayed hidden.

Fix the misfiling instead: .S maps to sourcecode.asm.asm (preprocessed, which the
capability gate in the file needs) and .s to sourcecode.asm, and both route into
the Sources phase rather than Resources. Every future assembly file gets this too.

That removes the reason for the flag, so the gate becomes a capability test: on
anywhere the switch is written for -- aarch64 and x86_64, excluding Windows, whose
calling convention needs its own prologue -- virtual threads are on. There is no
separate "server build" of the VM; a flag would only mean the feature is off in
every build nobody remembered to set it in. Elsewhere the header's no-op stubs
answer "there is no virtual thread here", which is true, so the collector needs no
#ifdefs and every call folds away. CN1_DISABLE_VIRTUAL_THREADS forces that path.

The predicate is repeated verbatim in the .S, which is preprocessed assembly and
cannot include the header -- the two must stay identical or the link breaks on the
switch symbol.

Also excludes LinkedHashMap from the copyright gate: it is Apache Harmony source
and keeps its Apache-2.0 notice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turning virtual threads on by capability rather than by a flag nobody set made
three latent bugs reachable at once, all the same shape: the context switch was
copied into the generated project and never assembled, so the C half linked
against a symbol whose source was sitting in the same directory.

  - CMake globbed *.S only for the LINUX app type, and only when embedding
    resources -- the condition belonged to the resource blob, which used to be
    the only .S there is. Now any .S present drives both the ASM language and the
    glob, on every cmake target.
  - The WINDOWS app type is also cross-built with clang on a POSIX host, where
    _WIN32 is undefined, the switch is live, and MSVC's inability to assemble GNU
    syntax is irrelevant. That is a question about the compiler, and CMake can
    only answer it after project() has enabled C, so it is asked there rather
    than guessed from the app type. Under MSVC the variable stays unset and
    expands to nothing.
  - Xcode has no mapping for .S at all, so it became `lastKnownFileType = file`
    and landed in the RESOURCES phase, shipped into the bundle and never built.
    sourcecode.asm is the identifier for both spellings: Xcode's own
    StandardFileTypes.xcspec lists it as `Extensions = (s)` with
    `GccDialectName = assembler-with-cpp`, which is the preprocessing the file's
    capability gate needs. The neighbouring sourcecode.asm.asm is for .asm.

Tests. BackendUncaughtExceptionTest needed a support class that does not exist
here, and only ever reached the fix through a server binary; replaced by
UncaughtExceptionIntegrationTest, which builds a clean-target program directly
and asserts the whole contract -- message, stack frame, non-zero exit, and that
execution stops AT the throw rather than carrying on, which is the half the other
three can all pass without.

test_virtual_thread.c was built by nothing. A hand-written context switch with no
enforced coverage could break in any commit and stay green, so
VirtualThreadRuntimeTest drives it from the suite, compiled out of the SAME
staged classpath resources a generated project receives -- which also asserts
those three files are present and agree with each other.

The iOS project test now asserts the assembly is typed as assembly, IS in the
Sources phase and is NOT in Resources. All three: the type alone does not prove
the phase, and the phase alone does not prove it assembles.

The generator's own source set is what caught the last of it. Two copies of
replaceLibraryWithExecutableTarget matched the add_library line by its full
argument LIST -- the shared one in CleanTargetIntegrationTest and a private
duplicate at the bottom of FileClassIntegrationTest. Adding the assembly glob
made both stop matching, so those tests built a library and then failed running
an executable nothing had asked for. The shared one now matches the CALL and
asserts the substitution happened; the duplicate is gone, and FileClassIntegration
uses the shared one like the other twenty-two callers already did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All five were real. Taken together they are one theme: a virtual thread is a
mutator the collector cannot see by the usual means, and the code that creates
one was doing only half the job.

RUNNING VIRTUAL THREADS LOOKED PARKED. cn1SpawnVirtualThread builds its VM state
with bindToCallingOsThread false, which leaves threadActive FALSE, and nothing
ever raised it. A collection running concurrently therefore treated a mutator
executing Java as parked, and was free to scan or migrate its object stack and
pending-allocation table underneath it -- missed roots at best, corruption at
worst. The flag now moves with the context switch, up on resume and down on
suspend, because a SUSPENDED virtual thread genuinely is parked: the collector
reaches its roots through the registry snapshot instead.

The transition is a weak symbol with a no-op default, not a function pointer.
cn1_virtual_thread.c cannot include cn1_globals.h (the standalone runtime test
builds it with no VM at all), an indirect call on a path whose entire value is
that it costs 2.1ns is not free, and a weak symbol costs a direct call the linker
resolves to the VM's real one when there is a VM.

NOTHING RELEASED THE STATE. cn1VirtualThreadFree knows only about the coroutine.
The VM state spawned beside it holds a 264KB shadow stack, the call-stack arrays,
the pending-allocation table, and one of the NUMBER_OF_SUPPORTED_THREADS slots in
allThreads. A virtual thread per request would have consumed a slot per completed
request and eventually tripped CODENAME_ONE_ASSERT(threadOffset > -1). Added
cn1RetireVirtualThread, which marks the state dead the way an OS thread's death
does and then frees it with the same gcQueuedForDrain deferral the Java finalizer
uses.

THE UNCAUGHT-EXCEPTION EXIT WAS NOT GATED. This is the one that would have
shipped. The generated main() is emitted for every target that has one, iOS and
macOS included, and cn1AbortOnUncaughtException was set unconditionally -- so an
uncaught exception on any thread would have terminated a shipped app. The comment
sitting above it claimed the opposite ("Only this target opts in, so nothing that
ships today changes behaviour"), which was simply false: the enclosing guard is
`if(m.isMain())` and nothing more. Now gated on OUTPUT_TYPE_CLEAN.

BLOCKING STDIN NEVER PARKED THE MUTATOR. System.in.read() waits as long as nobody
types, with the thread left active, so a concurrent collection spun for a
safepoint that could not arrive until a human pressed a key. Bracketed with
CN1_YIELD_THREAD/CN1_RESUME_THREAD like the socket reads -- which then needs the
keep-alive those reads also need, because only an interior pointer into the array
is live across the call and the collector would otherwise sweep the buffer being
filled. Portable here (a volatile store) rather than the Linux port's asm
barrier, because this file also compiles under clang-cl. feof is read before the
resume for the same reason errno is: the resume is a safepoint, and anything
asked afterwards describes the wait.

THE SHADOW STACK WAS FREED THE WRONG WAY. cn1AllocThreadStack falls back to
calloc when mmap is out of MAPPINGS rather than out of memory, and
cn1FreeThreadStack always called munmap. That fails with EINVAL and leaks the
whole stack -- or, on an allocator that returns page-aligned blocks, unmaps
memory the allocator still believes it owns. Which allocator answered is now
recorded and the free is paired to it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… does

Mapper.Direct's contract is to produce exactly what
JSONWriter.toJson(toMap(instance)) would. Two fields did not, so a mapper changed
its wire representation on the day it gained a direct writer:

  - A null List serialised as `null`, where the map path emits `[]` --
    emitFieldToMap builds its ArrayList unconditionally and fills it only when
    the source is non-null.
  - Enum elements went through toString(). The map path uses Enum.name(), and
    deserialisation matches against the declared constants, so an enum that
    overrides toString() produced JSON that could not be read back at all.

Every other element kind was checked rather than assumed: appendJsonValue already
maps Date to getTime(), scalars and collections through writeJson, and a mapped
object through its own mapper -- the same three answers emitFieldToMap gives.

Nothing was comparing the two paths, which is why both got through. Every
existing test exercises one route or the other, never one against the other, so
the divergence was invisible to all of them. directJsonMatchesTheMapPathExactly
runs an object with a populated list, an enum list, a Date and scalars, and then
the same class with every list left null, asserting the two routes produce
identical text. It asserts equality of the paths rather than against a literal on
purpose: it keeps holding when a field kind is added, with nobody remembering to
extend a hand-written expectation.

Two things that test needed before it proved anything. It drives the generated
mapper's own toJson rather than Mappers.appendJson, which goes through the
registry -- unpopulated in an isolated classloader, so it fell back to toString()
and compared the map path against "com.example.Swatch@23706db8". And it asserts
the mapper actually implements Mapper.Direct, without which it would compare the
map path with itself and pass while testing nothing. The test enum deliberately
overrides toString() to disagree with name(), so the wrong choice cannot pass.

Also drops a redundant `public` on the interface: PMD's UnnecessaryModifier, and
a zero-findings gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ow needs

Two CI breakages, both from this branch making something reachable that had not
been reached before.

EVERY cn1lib NATIVE CHECK STOPPED AT A MISSING HEADER. cn1_globals.h now includes
cn1_virtual_thread.h -- CN1_RESUME_THREAD yields a virtual thread rather than
sleeping the carrier it runs on -- and two places stage the port headers into a
scratch directory to compile a cn1lib against them. Neither knew about the second
file, so both stopped at "'cn1_virtual_thread.h' file not found" before compiling
a line: the six ad-cn1lib xcodebuild probes and check-cn1lib-native-sources.py.
The workflow's path filters gain the header too, otherwise a future change to it
skips the very check that would catch this.

java.io.File HAD NO WINDOWS PATH. Its non-ObjC arm is POSIX-only -- unistd.h,
dirent.h, access(), X_OK -- and Windows reaches that arm under clang-cl, which is
neither __OBJC__ nor POSIX. It went unnoticed because java_io_File_runtime.c is
emitted only when an app actually uses java.io.File, and until the clean target
became a usable program runtime no Windows build ever did. Now every one of them
failed on 'unistd.h' file not found.

The Win32 arm: io.h and direct.h for _access, the access-mode constants the MSVC
CRT does not define, and FindFirstFile for the directory walk, in the same
two-pass shape as the POSIX one (count, allocate, refill) because allocArray can
collect and the array must not be built with a find handle open. X_OK maps to an
existence check: Win32's access model has no execute bit, and _access REJECTS a
mode of 1 rather than answering "not executable". isHidden asks for
FILE_ATTRIBUTE_HIDDEN instead of guessing from a leading dot, which means nothing
on Windows. Everything else -- stat, remove, rename, mkdir -- the CRT already
provides under the same names.

Also merges two identical project() branches that SpotBugs flagged as
DB_DUPLICATE_BRANCHES: Linux and the clean target answer the assembly question
the same way, so they share one branch instead of two spelled alike.

The POSIX arm is verified here (FileClassIntegrationTest, 5/5); the Win32 arm can
only be verified by CI, which is what reported it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeQL java/zipslip, high severity. unzip() built each output path by
concatenating the destination with ZipEntry.getName(), unchecked, so an entry
named "../../x" wrote wherever the archive asked. Both callers unpack a
DOWNLOADED zip -- Groovy for the console, JavaFX for the browser component -- so
the archive is not something the user authored, and the consequence is an
arbitrary file overwritten under their account while they believe they are
unpacking a dependency. CWE-22.

Every entry now has to resolve inside the destination or it is refused. The
comparison is between CANONICAL paths -- resolving the ".." is the whole point --
and it uses java.nio.file.Path.startsWith rather than String.startsWith, for two
reasons. Path compares COMPONENT-wise, so a sibling like "/tmp/dest-evil" is
rejected against "/tmp/dest" where a character-wise prefix accepts it, and giving
the string prefix a trailing separator to fix that then wrongly rejects the
destination directory itself. It is also the shape CodeQL recognises as a
sanitizer: the first attempt here was a correct canonical-path check that the
query still flagged, because a compound `!a && !b` guard did not read as a
barrier.

Two things the fix had to bring with it, both found by writing the test:

  - Parent directories are created before extracting. FileOutputStream will not
    create them, and a nested entry can arrive before the directory entry that
    holds it, so "nested/deep/leaf.txt" in an archive that declares no directory
    entries threw FileNotFoundException. That was broken before this change too.
  - destDir uses mkdirs rather than mkdir, so a destination more than one level
    deep is actually created.

Both streams are closed in a finally, which they were not: an IOException
mid-extract leaked the descriptor.

The test builds the malicious archive rather than checking one in -- a committed
zip that escapes its destination is an awkward thing to keep in a repository, and
building it puts the attack in front of the reader. Verified non-vacuous by
reverting the fix: 2 failures against the old code, 0 against the new.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing the unistd.h/dirent.h dependency got clang-cl past the first error and
into four more, all the same kind -- POSIX spellings the MSVC CRT does not have:

  - `redefinition of 'timeval'`. <windows.h> pulls in <winsock.h>, whose timeval
    collides with the one cn1_win_compat.h defines. WIN32_LEAN_AND_MEAN keeps
    winsock out, and nothing here wants it.
  - S_ISDIR / S_ISREG undeclared. The CRT has the st_mode BITS but not the macros
    that test them, so they are defined from _S_IFMT/_S_IFDIR/_S_IFREG.
  - PATH_MAX undeclared -- MAX_PATH is the Win32 spelling.
  - realpath undeclared. _fullpath is the equivalent, but it takes
    (destination, source), the REVERSE of realpath's (source, destination), so
    the macro swaps them. Getting that backwards compiles and canonicalizes the
    wrong string in silence. It also resolves a path that does not exist rather
    than failing, which is the more useful answer for getCanonicalPath.

The POSIX arm is unchanged and still verified here (FileClassIntegrationTest,
5/5). The Windows arm is verified only by CI, which is what reported both rounds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two Windows-only build breaks in this branch's own new code, both invisible on
the POSIX legs.

`i->gcPthread = 0` for a virtual thread's state is a type error under clang-cl:
pthread_t is a POINTER on Apple and glibc, but the Windows compat shim defines it
as struct {handle, id}, so the assignment reads as "assigning to 'pthread_t' from
incompatible type 'int'". memset over sizeof is correct for both shapes, and
gcPthreadValid -- set FALSE on the next line -- is what actually gates every read
of the field.

cn1AllocThreadStack declared its byte count above the #if that uses it, so on
Windows, whose arm calls calloc with the element count instead, it was an unused
local. Moved onto the arm that uses it.

Swept the rest of this branch's additions for the same class of thing rather than
waiting for CI to find them one at a time: every other POSIX call in code Windows
compiles is either guarded (mmap/munmap behind !_WIN32, pthread_attr_setstacksize
behind __linux__) or shimmed in cn1_win_compat.h (usleep, pthread_key_create,
pthread_getspecific). The virtual-thread runtime -- including the
__attribute__((weak)) definition, which clang-cl treats differently on COFF -- is
entirely inside the CN1_VIRTUAL_THREADS gate, which excludes _WIN32, so none of
it is compiled there at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A null array crashed instead of throwing (P1). CN1_ARRAY_STORE_CHECK evaluates
CN1_CLASS_OF(arrayObj) with no null guard, and under -Dcn1.checkedCasts it runs
AHEAD of the setter that turns a null array into a NullPointerException -- so an
object-array store through a null array took the process down. Java orders NPE
ahead of ArrayStoreException anyway, so falling through to the setter is both the
safe answer and the correct one.

A virtual thread's stack could go unmarked mid-switch (P1). The parked-stack pass
skipped anything cn1VirtualThreadIsRunning() reported, on the reasoning that the
carrier covers those. It does -- but only once the carrier's stack pointer is
actually INSIDE the virtual stack, and `running` is raised before the switch and
lowered after the switch back. In those two windows a stopped carrier still has an
OS-stack pointer, so cn1VirtualThreadForStackAddress matches nothing, the carrier
pass scans only the OS stack, and this pass skipped the virtual stack for being
"running". References held in C temporaries there could be swept.

The flag cannot be made atomic with the switch it brackets, because the switch is
what changes the stack the flag would have to be written from. So the passes now
OVERLAP instead of partitioning: every virtual thread's saved region is scanned
unconditionally. Safe, because [sp, stackHigh) is inside the mapping whenever sp
is non-zero; complete, because while a virtual thread runs the carrier's pointer
is lower, so this pass covers a subset and the carrier covers the rest; and cheap,
because conservative marking is idempotent.

cn1RetireVirtualThread's "use after free" was NOT one, and the code now says so.
markDeadThread -> collectThreadResources sets gcQueuedForDrain unconditionally and
has no early return, so the synchronous release branch was unreachable. It read as
live, though, so it is gone and the invariant is written down -- including the
reason it matters, which the report had right: codenameOneGCMark copies each
ThreadLocalData* out of allThreads under the critical section and dereferences it
OUTSIDE the lock, so a synchronous free would be a genuine use-after-free.

File.list returned something that called itself a String. All three arms passed
the ELEMENT class to allocArray, which installs whatever it is given as the array
object's own class; cn1MainArgs has always passed class_array1__java_lang_String.
Pre-existing on iOS and Linux, copied into the new Windows arm, fixed on all three.

Windows absolute paths were treated as relative, which corrupted them rather than
merely misreporting them: getAbsolutePathImpl tested p[0] == '/', so "C:\data"
had the working directory prepended. There is now a per-platform predicate that
knows about drive letters and UNC roots. The matching Java-side gap is deliberately
left and documented at the predicate: File.isAbsolute() tests
startsWith(File.separator) and separator is "/" everywhere, which needs a
per-platform separator in shared JavaAPI -- a change for every port, not for
making the clean target build.

Blocking file reads and writes now park the mutator, like the socket reads and
StandardInputStream already did: a FIFO, a device or a network-backed path blocks
for as long as the far end stays quiet, and an active thread there strands the
collector waiting for a safepoint that cannot arrive. Both carry the buffer
keep-alive for the same reason those do -- only an interior pointer is live across
the call. (Moving that macro above its first use is why it now sits at the top of
the file layer rather than beside stdin.)

The benchmark helper compiles the emitted .S. Third place with this bug: the CMake
generator and the Xcode project generator had it too, and a *.c-only invocation
links against a missing cn1VirtualThreadSwitch on any target where the switch
exists.

Two findings are recorded in the file rather than fixed, with the analysis and the
actual remedy: 32-bit ftell/fseek cannot express a position past 2GiB where C long
is 32 bits, and paths reach the narrow CRT as UTF-8 and are read as ANSI. Both are
pre-existing on every platform, both want a change across the whole file layer,
and neither is what enabling the clean target is about.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mapper.Direct promises identical output, not better output. Each of these was the
direct path being reasonable in a way emitFieldToMap is not, which is the same
thing as changing a mapper's wire format the day it gains a direct writer.

  - A property NAME was escaped for the Java literal and not for JSON. escape()
    doubles a quote so the generated source compiles; the resulting writer then
    appended the raw character, so a @JsonProperty holding a quote emitted
    "a"b" -- unparseable. The map path never had this because JSONWriter puts the
    key through writeString. Now jsonEscape composed with escape: one makes the
    JSON valid, the other makes the source compile. Done at generation time, since
    a jsonName is a compile-time constant and the writer should stay a literal
    append.
  - A Property value was rendered too well. emitFieldToMap stores it RAW, so
    JSONWriter renders a Date or a mapped object through String.valueOf;
    appendJsonValue turned them into epoch millis and nested JSON. New
    Mappers.appendJsonRaw is exactly JSONWriter's answer for a value that was put
    in the map unchanged.
  - A reference field looked its mapper up by RUNTIME class. A field declared as a
    mapped base holding an unmapped subclass therefore found nothing and fell back
    to a quoted toString, where the map path asks Mappers.get(Declared.class) and
    serialises it as an object. New Mappers.appendJsonUsing takes the mapper the
    caller names, and still uses that mapper's direct route when it has one.
  - Mapped list ELEMENTS had the same problem, plus the general one behind it: the
    direct path had a two-way branch where emitFieldToMap has four. It now mirrors
    them one for one -- enum name(), scalar raw, Date getTime(), everything else
    through the declared element type's mapper.

The test was the actual defect. Nothing compared the two paths against each other,
which is why all of this shipped; and the parity test added for the first pair
needed three fixes of its own before it proved anything:

  - It went through Mappers.appendJson, which consults the registry. In an
    isolated classloader the registry is empty, so it compared the map path
    against "com.example.Swatch@23706db8". It now drives the generated writer.
  - The polymorphic case had no mapper registered for the base type, so BOTH paths
    fell back to toString and agreed. Registering it is what makes the two
    implementations able to differ at all.
  - assertEquals reports the FIRST difference, so one unfixed case masked the
    others. Each representation is now pinned individually, which also catches the
    case equality cannot: both paths wrong in the same way.

Verified by reverting the generator with the test in place: one failure against
the old code, six passing against the new.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more review findings, both in code this branch touched.

skip(Long.MAX_VALUE) computed `start + count` and clamped afterwards. Once any
byte has been read that addition overflows signed long -- undefined behaviour, and
in practice a wrap to negative, so the seek goes BACKWARDS and the caller is told
it skipped a negative distance or gets an error where it should have landed on
EOF. It now clamps against the remaining DISTANCE, which cannot overflow: end is
at least start, and start plus the clamped amount is at most end.

File.list walked the directory TWICE -- count, allocate, walk again -- and assumed
both walks saw the same directory. They do not. A file created in between overruns
the array, and CN1_SET_ARRAY_ELEMENT_OBJECT turns that into
ArrayIndexOutOfBoundsException; a file removed leaves trailing nulls in a String[]
that no caller expects. Directories change under readers routinely, so this was
never sound. I wrote the Windows arm that way deliberately, mirroring the POSIX
one, which means I copied the structure without asking whether it held.

Both arms now enumerate ONCE into a small growable list of names and build the
array afterwards. The names are held in C memory on purpose: allocArray and
newStringFromCString can both collect, and nothing may hold a directory handle
across that. The ObjC arm is left alone -- NSFileManager hands back a snapshot, so
it never had the race. Also moves stdlib.h to the shared include group, since the
list uses malloc/realloc/free on both arms and sits outside the platform blocks.

The test is the part worth reading. FileClassIntegrationTest never called
File.list(), so the native listing was COMPILED but never RUN by any suite: the
rewrite above passed 5/5 while executing none of it, and reverting it would have
passed too. Coverage now creates a directory, lists it, and pins the three things
that were wrong or fragile -- the entries, the absence of nulls, and that the
result is a String[] rather than a String, which is the pre-existing allocArray
class bug nothing had ever asserted.

Confirmed the assertions discriminate rather than merely execute: with the array
class reverted to the element class, all five configurations FAIL; restored, all
five pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more findings, both consequences of this branch making java.io.File usable on
Windows.

"C:foo" is DRIVE-RELATIVE: relative to the working directory of drive C, which is
not the process working directory and may be on a different drive. cn1FileIsAbsolute
classified it correctly -- the comment there even says so -- and then the fallback
prepended the process cwd anyway, producing "D:\cwd\C:foo", which names nothing.
The predicate knew about a case the code after it did not. _getdcwd asks the right
drive.

Deliberately not _fullpath, which the report suggested: it also normalises "..",
and getAbsolutePath is specified NOT to do that -- resolving is getCanonicalPath's
job. Using it would have swapped a wrong path for a subtly wrong contract.

createNewFile was check-then-act: access(), then fopen(p, "w"). Losing that race
does not merely return the wrong answer, it TRUNCATES the file the other process
just created, and then reports true as though it had done the creating -- which is
exactly the failure mode the lock-file and single-instance patterns it exists for
cannot survive. Now a single O_EXCL open on both arms, with the kernel deciding.
Pre-existing on POSIX too, so both are fixed.

ON THE TEST, because the distinction matters: the coverage added here is a
REGRESSION GUARD, not a demonstration of atomicity. It checks the uncontended path
-- createNewFile on an existing file returns false and leaves it intact -- and the
old check-then-act version passes it too, because access() succeeds and it returns
before reaching the truncating fopen. Confirmed by running the suite against the
old implementation: 5/5 green. The real defect needs a file to appear between the
check and the open, which one thread cannot arrange, so the argument for the fix is
structural rather than empirical and the comment in the test says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
newStringFromCString turns each byte into its own char. That is correct for what
it exists to serve -- generated string literals, which are ASCII plus ~~uXXXX
escapes -- and wrong for anything arriving from outside the program. A UTF-8
"e-acute" is two bytes, so main(String[]) and System.getenv handed back one
garbage char per byte, corrupting paths and option values before the program had
a chance to look at them. Both entry points are new in this branch.

newStringFromUtf8 decodes properly: multi-byte sequences, surrogate pairs for
astral code points, and U+FFFD for malformed input the way java.lang.String's own
decoder does -- a program should not die because one environment variable holds a
stray byte. Overlong forms, UTF-8-encoded surrogates and out-of-range code points
are all rejected.

newStringFromCString itself is deliberately NOT changed. Every native-to-Java
string in the VM goes through it, its byte-widening is load-bearing for the
literals it serves, and its own comment records that the high-bit path is
bit-identical to what came before. Correcting the two entry points this branch
added is the scoped fix; the general version is the same work as the ANSI-versus-
UTF-8 path issue already recorded in nativeMethods.m.

TWO BUGS UNDERNEATH, both found by the test rather than by reading:

newString was broken and had never been called from C. JAVA_CHAR is an int and
JAVA_ARRAY_CHAR is an unsigned short, and it sized the allocation with
sizeof(JAVA_CHAR) while memcpy'ing length * sizeof(JAVA_ARRAY_CHAR) bytes out of a
four-byte-element array -- half the input, at the wrong stride. My decoder was its
first caller and hit it immediately: "cafe" came back as c,NUL,a,NUL,f. It now
narrows element by element.

Behind that, the representation is not a free choice. A string whose units all fit
in a byte is stored as a COMPACT byte[], anything else as a char[], and charAt
reads whichever it finds -- so handing it the wrong one reads 8-bit units out of
16-bit data and produces exactly the same symptom rather than failing. That rule
now lives in cn1StringFromUnits, used by newString and newStringFromUtf8.
newStringFromCString keeps its own copy on purpose: it tracks the Latin-1 flag
during decoding and runs for every literal at startup, so routing it through a
helper that recomputes would add a pass over every literal in the program to save
a dozen lines. The comment says so, and says the two must change together.

The test reports CODE POINTS rather than text, so it cannot pass through a
console-encoding coincidence: "cafe-acute-euro" must arrive as 99,97,102,233,8364,
which covers a two-byte and a three-byte sequence. Byte-widening reports the
individual bytes instead, which is how the newString bug surfaced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s UTF-8

The Windows clean-target leg failed the test added with the UTF-8 decoder, and it
was right to: "cafe-acute-euro" arrived as 99,97,102,65533,65533 -- c, a, f, and
two replacement characters. The CRT hands main() and getenv() the wide command
line and environment already converted down to the ACTIVE CODE PAGE, so decoding
those bytes as UTF-8 finds invalid sequences and substitutes U+FFFD for every
non-ASCII character.

That failure was predicted by a comment I had written in this very function --
which then shipped alongside a test asserting the behaviour the comment said did
not exist. MultiByteToWideChar with CP_ACP is the conversion Windows actually
needs, and it yields UTF-16 code units directly, so nothing decodes afterwards.

RENAMED from newStringFromUtf8 to newStringFromNative for the same reason: a
function named FromUtf8 that deliberately does not decode UTF-8 on one of its
platforms is a trap for whoever reads it next. The name now says what it does --
convert text that came from the OS, in whatever encoding the OS used.

WIN32_LEAN_AND_MEAN before windows.h, which is the same winsock timeval collision
that broke java_io_File.m; and the byte-length local moved onto the POSIX arm,
which is the only one that uses it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JLS orders these: NullPointerException, then ArrayIndexOutOfBoundsException,
then ArrayStoreException. Under -Dcn1.checkedCasts the emitted covariance check
ran BEFORE the setter that reports the first two, so a store with both a bad index
and an incompatible value reported the value -- hiding the exception the program
should have seen. (The null case was worse and is already fixed: the check
dereferenced the array to reach its class.)

The store check is now guarded by the same access validation the setter performs,
so the first two exceptions are thrown first and in the right order. The setter
re-checks, which on the in-bounds fast path costs one comparison.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e collector

This backs out my own fix from earlier in this branch. Marking the attached
ThreadLocalData threadActive around the context switch reads as obviously correct
and is a REGRESSION, worse than what it fixed.

A virtual thread's state has no pthread of its own -- deliberately, it may run on
a different carrier next time. The collector's wait for a lightweight thread is
`while(t->threadActive) usleep(500)` with no bound, and the forced-stop escalation
that exists to break exactly that wait is gated on gcPthreadValid, which is
permanently false here. So the flag converts a POSSIBLE race on the state's object
stack into a CERTAIN hang for any virtual thread that computes without reaching a
safepoint: the collector waits for a flag only that thread can clear, and cannot
stop it.

What the same report asked for has two halves, and the other one stands. The C
stack is covered: cn1GcScanParkedVirtualThreads scans every registered virtual
thread whether or not it is running, so no virtual stack goes unscanned during the
windows where `running` is set but the carrier has not switched yet. That fix is
independent of this revert and stays.

The half that remains open -- a collection walking the state's object stack and
pending-allocation table while the virtual thread mutates them -- is documented at
cn1SpawnVirtualThread along with why the obvious fix is worse and what the real one
is: carrier association. A running virtual thread executes ON a carrier that does
have a stoppable pthread, so the collector should satisfy the wait by stopping the
carrier. That needs the stop handshake to stop being per-TLD (the signal handler
records into the TLD of the thread it runs on, which is the carrier's), i.e. a
change to the collector's stop protocol rather than to the spawn path -- not
something to improvise in an API that has no callers yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With -Dcn1.checkedCasts the covariance check broke correct programs, which is the
worst direction for a check to fail in. A generated array class records arrayType
as the BASE element class rather than the immediate component: String[][] has
dimensions 2 and arrayType String, not String[]. So `values[0] = new String[1]`
asked whether a String[] is an instance of String, got no, and threw
ArrayStoreException on a store the language requires to succeed.

Restricted to dimensions == 1, where arrayType genuinely IS the component type.
Multidimensional stores lose a diagnostic that did not exist before this feature
was added; the alternative was breaking working code. Covering them properly needs
the immediate component type, either emitted per array class or reconstructed from
dimensions at runtime, and the macro says so.

Also fixes a timeout in VirtualThreadRuntimeTest that could never fire. It read
the child's output inline and then called waitFor: the read blocks until the child
closes stdout, so a binary that hangs -- exactly what a context-switch regression
produces -- never reached the timeout, and the Maven job would sit until CI killed
it instead of the test failing. Output now drains on its own thread, with a bounded
join so a wedged reader cannot reintroduce the hang the change removes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This corrects my own change earlier in this branch, and the reasoning behind it
was the defect. "A thread it cannot stop is one it does not scan either way" is
true only while the thread genuinely cannot be stopped. Failures are often
TRANSIENT -- a stop signal briefly masked is enough -- and the thread recovers.
Skipping it then meant cn1GcScanThreadNativeStack returned without scanning a
RESPONSIVE thread, for roughly the next sixty collections, so references held only
in frameless C locals or registers went unmarked and could be reclaimed while
still in use. A GC correctness bug, traded for a performance win.

The two things I had conflated: the cost was never the SIGNAL, it was the WAIT.
One unresponsive thread consumed the entire 2,000,000-spin budget -- 267ms of a
280ms mark. So a thread with a failure history is now probed with a 20,000-spin
budget rather than skipped. Healthy threads answer within about 200 spins, which
is a hundredfold margin for one that is merely slow, at one percent of what a hang
used to cost; and a thread that recovers is picked up on the very next cycle
instead of up to 64 later.

Verified across the GC suites, including GcUncooperativeThreadIntegrationTest --
the issue #5537 scenario this logic exists to serve: 6/6.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Boolean shares its kind with boolean and Character with char, so the direct writer
treated both as primitives. Only the boxed form can be null, and both handled it
wrongly in opposite ways: a null Boolean was unboxed by a ternary and threw
NullPointerException, and a null Character went through String.valueOf(Object),
which returns the four characters "null", and was then QUOTED -- so an unset field
serialised as the string "null". The map path stores the value and lets JSONWriter
see the null, emitting JSON null for both.

Told apart by binaryName, which does distinguish them, with a temporary in each so
a getter is not evaluated twice, and charValue() so String.valueOf resolves to the
char overload rather than the Object one.

The parity test carries both fields now, and they discriminate by construction:
against the old code the Boolean case throws (a test error) and the Character case
produces a quoted "null" against the map path's null (an assertion mismatch).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CODENAME_ONE_ASSERT is plain assert(), which NDEBUG compiles out of every release
build. So once all NUMBER_OF_SUPPORTED_THREADS slots were taken, threadOffset
stayed -1, the assertion vanished, and the next statement executed
allThreads[-1] = i -- writing over whatever precedes the table. A debug build
aborted; a shipped one carried on with silent memory corruption, which is the
worse of the two. Capacity exhaustion is a condition to report, not to assert.

It returns 0 now, and cn1SpawnVirtualThread already checks for that.

Pre-existing rather than new: every OS thread creation runs this path too. A
virtual thread per request only makes reaching the limit realistic.

The partially built state is unwound through cn1FreeThreadLocalDataFields,
extracted from cn1ReleaseThreadLocalData rather than copied, because the release
path also decrements nThreadsToKill and a state that never reached allThreads was
never counted as living. Duplicating the frees would have drifted apart, and
getting that counter wrong would have been a slow leak in the opposite direction.

Verified across the GC suites including GcUncooperativeThread and GcHeapIntegrity:
6/6. (The translator build says nothing about this -- it compiles Java, and the C
here is only compiled by those tests.)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…reeing

Two defects, and both are mine from earlier in this branch.

THE HANG I REVERTED WAS STILL REACHABLE. Removing the threadActive assignment from
cn1VirtualThreadResume did not close it, because CN1_RESUME_THREAD does the same
thing and every bracketed native goes through that macro. getThreadLocalData()
resolves to the VIRTUAL thread's state while one is running, so a virtual thread
that read a file or a socket returned with its state marked active, and nothing
lowers it again until the next yield. Same unbounded while(threadActive) wait, same
forced-stop escalation gated on gcPthreadValid and therefore unavailable, same
stall. I checked the call site I had edited and not the shared path through it.

The guard states the invariant the code always needed: mark active only what the
collector can STOP. gcPthreadValid is exactly that question. A real thread is
unaffected; a virtual thread's state stays down, which is where it was before any
of this. Roots do not depend on the flag -- cn1GcScanParkedVirtualThreads scans
every registered virtual thread whether or not it is running.

THE EXHAUSTION CHECK INTRODUCED A USE-AFTER-FREE. pthread_setspecific binds the new
state to TLS above the capacity search, so the failure path I added freed a state
the key still pointed at: every later getThreadLocalData() on that thread would
return memory that had been given back. That is worse than the out-of-bounds write
it replaced, because the thread keeps using the stale pointer rather than failing.
Unbound before the free.

Also: System.getenv(null) throws NullPointerException as the API requires, instead
of returning null and making an invalid argument indistinguishable from an unset
variable.

Verified across the GC suites, 6/6, including GcUncooperativeThread and
GcHeapIntegrity.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
  * Both processors refuse to generate a class whose name is taken, and
    read their OWN previous output as such a class. The second pass of an
    incremental build -- process-classes again, without a clean -- scans
    target/classes, finds the router or the dispatcher the first pass
    wrote, and reports a collision. Every project using @RestController
    failed its second build; a contract could be processed exactly once
    per clean output directory. Generated classes carry a @generated
    marker now and the guard skips them. A hand-written class of the same
    name has no marker and is still refused -- there is a test for that,
    because a marker that turns the guard off entirely would be worse
    than the bug.

  * A route shape collapsed each segment holding a placeholder to "{}",
    so "/{name}.json" and "/{name}.xml" were the same shape. The literals
    are part of it now.

    That was not the whole refusal. The server kept its OWN copy of the
    segment-overlap rule, and the copy still called every pair of
    variable-carrying segments a collision -- the exact thing fixed in the
    controller processor two commits ago. So a contract the matcher
    handles was refused here and accepted there. There is one
    implementation of that rule now, because a rule copied is a rule that
    gets fixed once.

  * A contract path without a leading slash never matched. The client
    resolves it against a base URL and requests /notes; the server split
    the template to one segment against the incoming two, so the route
    answered nothing. Templates are normalised to origin-form before
    splitting, which also fixes the empty template against "/".

  * Crypto.hashPassword(null) produced a valid verifier. utf8(null) is an
    empty array, so a handler passing a DTO field the client never sent
    created an account that verifyPassword("", ...) opens.
    verifyPassword already refused null; this is the other half, in both
    runtime arms, asserted by the self-test that runs on both.

The marker had to ship in the backend artifact for generated sources to
compile against it, which is why maven/backend is rebuilt here rather
than only the plugin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0ed3163ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/HttpServer.java Outdated
CN1_HTTP_MIN_BODY_RATE is a divisor in both body readers, and nothing
stopped it being 0. It is clamped now, along with CN1_HTTP_VT_STACK (a
stack size) and CN1_HTTP_TARGET_CACHE (an array size, where 0 is a
documented "disabled" and only a negative is nonsense). A rejected value
says so on stderr and the default is used.

The interesting part is what a zero divisor actually does, which is not
what the report predicted and not the same on the two runtimes:

    int zero = Integer.parseInt("0");  1000 / zero

    Java SE    threw ArithmeticException
    ParparVM   answered 0

Measured, not assumed. So the Java SE dev loop drops the connection with
no response -- the reported behaviour -- while the packaged binary
quietly loses the rate term of its own deadline and carries on serving.
That is worse than a crash in one respect: the difference is invisible
until an upload that should have been granted time is cut off with a 408
in production and cannot be reproduced locally.

It also means a behavioural test cannot catch this on both arms, because
one of them barely misbehaves. A first version of the test sent a body in
one write, which fillTo() answers before it ever computes the allowance,
so it passed with the guard removed. The test asserts on the clamp's own
stderr line instead, which both arms print and the fixture captures --
and the fixture now runs with CN1_HTTP_MIN_BODY_RATE=0 on purpose, so
every upload test on that port carries the proof.

The clamp is deliberately NOT reachable from the self-test: SelfTest is
in com.demo, and making the helper public to test four lines of obvious
arithmetic would widen the API for a test's convenience.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6be74d91b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/HttpServer.java
Comment thread vm/backend/src/com/codename1/backend/HttpServer.java Outdated
Comment thread vm/backend/src/com/codename1/backend/StaticFiles.java Outdated
Comment thread vm/backend/native/cn1_backend_http2.c Outdated
… root

Three of the four findings in this round were real. The P1 was not, and
finding that out took longer than fixing the rest.

  * NOT a leak. The report was that an HTTP/2 HEAD of a static file never
    closes its descriptor. responseBodyFor() closes it in a finally, and
    its own comment says the HTTP/2 caller "comes here for HEAD alone,
    where the point of this branch is the close below". Adding a close in
    the bodiless branch as well made it a DOUBLE close -- measured at
    exactly one extra per request -- which is worse than the reported
    bug, because a descriptor number is reusable the moment the first
    close returns and the second lands on whoever took it. The reasoning
    is now a comment there, since the next reviewer will read the same
    branch the same way.

    It took three runs to see, because the metric and the test were both
    wrong in the same direction: the count ran to -10 and the test's
    reader scanned for digits only, so it parsed -10 as 10 and reported
    the leak it was looking for. A measurement that discards the
    character which disproves the hypothesis is not a measurement.

  * openStaticFiles is reported now, and means "descriptors this side
    still has to close". Nothing counted them before, the process limit
    here is over a million, and a real leak would surface hours later as
    a server that cannot accept sockets with nothing pointing at the
    cause. Ownership transfers are recorded at the transfer: a body
    handed to an HTTP/2 session is freed natively and is counted by
    Http2.pendingBodyFiles() from then on -- conflating the two made an
    ordinary h2 GET look like a leak, which is how the distinction got
    noticed.

  * The 400 for a malformed UTF-8 body ignored respond()'s answer, so
    under a full body budget the stream was left unanswered until the
    connection timed out. It falls back to a bodiless 400.

  * The HTTP/2 file-descriptor ceiling was checked in Java and taken in
    C, so every worker finishing at once passed before any incremented --
    the same check-then-act already fixed for the byte ceiling, left
    behind on the descriptor one. Reserved natively now, in the step that
    takes the descriptor, and the caller closes the fd when refused.

  * StaticFiles' containment check required root + "/". FileIo.realPath
    answers backslashes on Windows and openBeneath() is unsupported in
    the Java SE runtime, so every ordinary child reached that fallback
    and was answered 403: static files simply did not work in a Windows
    dev loop. Either separator is accepted; "/srv/wwwroot-evil" is still
    not inside "/srv/www".

The descriptor test pins BOTH directions: +10 is the leak the review
predicted, -10 is the fix for it, and 0 is the answer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d8dd0304b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/impl/parparvm/com/codename1/backend/Tcp.java
…tive timeout

  * %ZZ and %2 decoded as literal text, so the handler was given a value
    no client can have written. The aliasing is the part that matters:
    %252F is a correctly escaped %2F, and once a bad escape passes
    through as text, a check written against one spelling is defeated by
    the other -- and an intermediary that rejects or normalises the
    invalid form no longer agrees with this server about what was asked
    for. StaticFiles.decode has refused this all along; these were the
    copies that did not.

    Validated once at the entry point rather than inside the matcher.
    bindFrom() answers a boolean, so a decoder that refused would only
    turn a syntax error into "no route" -- a 404 for something the client
    could fix if told.

    The contract decoder had a second hole: Integer.parseInt(_, 16)
    accepts a sign, so "%+1" decoded to the byte 1 and "%-1" to -1. Two
    hex digits, tested as digits.

  * Tcp.connect forwarded a negative timeout to the native side, which
    reads every non-positive value as "block with no deadline". Java SE
    fails immediately out of Socket.connect, so the same call failed fast
    in the dev loop and hung a packaged server for the OS TCP timeout.
    Refused now, in the arm that diverged; zero keeps its documented
    meaning. Database's URL parser already refused this one layer up --
    this is the API a caller can reach directly.

Both halves of the escape rule are pinned, including that %41 still
decodes to A: a guard that refused everything would pass a test written
only against the bad input.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bcc4c88ba9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/impl/parparvm/com/codename1/backend/Tcp.java
Comment thread vm/backend/impl/javase/com/codename1/backend/Crypto.java
  * Tcp.read with length 0 answered 0 on Java SE -- InputStream's
    contract, inherited -- and END OF STREAM once packaged, because the
    native maps recv(_, 0)'s zero-byte result to -1. A caller that
    computed an empty slice was told the peer had gone away, but only in
    production. SSL_read(_, 0) is worse still: OpenSSL leaves it
    undefined. Answered before dispatching now, on both. write got the
    same guard: it is harmless today only because its check is
    n != length and 0 != 0 is false, which is a reason to be explicit
    rather than to rest on it.

  * pbkdf2Sha256 with a non-positive iteration count returned the
    ONE-ROUND result on Java SE instead of failing. The native has always
    refused it, so a misconfigured SCRAM or key derivation produced a
    weak key that appeared to work locally -- and a different key from
    the one the packaged server derives. Both arms refuse it now, and
    length <= 0 with it, which the native also refuses.

  * A controller returning a DTO that inherits Json.Writable from a
    superclass, or implements a subinterface of it, failed to compile.
    Json.writeValue asks `instanceof Writable`, which honours the whole
    hierarchy; the check read only the directly declared interfaces. So
    the build refused what the runtime encodes correctly, which is the
    worst direction for a build-time check to be wrong in. It walks
    superclasses and interfaces now, each visited once, and a type with
    no Writable anywhere is still refused -- there is a test for that
    side too.

The zero-length read is asserted against a real server rather than
argued about, which is also how I found that stop() without a drain
bound does not return here: the self-test sat past ten minutes before I
noticed I had called the wrong one of the two. It uses stop(1000), like
the probe above it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65b16e1d35

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/impl/parparvm/com/codename1/backend/Db.java Outdated
…ross jars

  * SQLite serializes each API CALL on a connection, not a BEGIN/body/
    COMMIT sequence -- and the demo asserted the opposite in a comment:
    "one shared connection is correct here, and SQLite serializes it".
    Four threads sharing one in-memory Db lose three of them to "cannot
    start a transaction within a transaction" and write 30 of 120 rows.
    Measured, not argued: that is what the new self-test check reports
    with the synchronization removed.

    execute, query and transaction now hold the connection's monitor,
    which is reentrant, so transaction() keeps it across the whole
    callback and the executes inside re-enter freely. A pooled connection
    is used by one thread at a time anyway, so it pays an uncontended
    lock; a shared one is serialized, which is what correctness requires.
    Both arms, and the demo's claim is corrected.

  * transferredFields() promised a DTO's inherited fields and stopped at
    the first superclass this build did not compile, because lookup()
    sees only the project's own output. A DTO extending a dependency's
    class lost every inherited field from toMap() and fromMap() --
    silently, on both ends, so the two agreed about a value neither sent.

    Fixing that alone would have made things WORSE: the generated codec
    then names the dependency's types, and this processor compiled its
    generated sources against the output directory alone. A silent
    omission would have become a build failure. The compile classpath is
    included now, as the controller processor already did.

  * A parameter carrying two binding annotations silently bound whichever
    the priority chain reached first. For @RequestHeader("Authorization")
    next to @RequestParam("token") that is the difference between a
    header a proxy controls and a query string the caller writes, and the
    declaration named both so no reader could tell which won.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da0fd68ec9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/HttpServer.java
Comment thread vm/backend/src/com/codename1/backend/HttpServer.java
…aped UTF-8

  * Every catch in serveOne is `catch (Exception)`, and an Error is not
    one. A StackOverflowError out of a recursive parser -- which this
    server already has a test about -- or an AssertionError from a
    handler walks past all of them and leaves serveOne without reaching
    any drop(). The descriptor has been removed from its poller by then
    and is still in liveConnections, so nothing will ever close it: one
    stranded socket per occurrence. serveOne is now a wrapper that
    releases and rethrows; a wrapper rather than a try around the body
    because the body has many returns and the point is that every one of
    them is covered.

  * Two hex digits is not enough to call an escape valid. %C3%28 is a
    truncated two-byte sequence, and new String(_, "UTF-8") answers
    U+FFFD instead of failing -- so the handler received exactly what
    %EF%BF%BD%28 produces. One value, two spellings, which is the same
    aliasing the escape-syntax check was added for one level up. Both
    generators validate the decoded bytes as UTF-8 now (RFC 3629, so
    overlong forms, surrogate halves and anything above U+10FFFF go too),
    and there is a test that %C3%A9 still decodes to one accented letter,
    because that is the direction this kind of guard breaks.

The third finding in the batch is REFUSED, and the reasoning is now a
comment on the constructor it names. It said the HTTP/2 Request leaves
pathLength at Java's default 0, so every generated route 404s over
HTTP/2. pathLength carries a field initializer (= -1), and javac copies
those into every constructor: that constructor's bytecode opens with
iconst_m1/putfield pathLength. Checked with javap rather than by reading,
because the sibling constructor assigns it again explicitly and this one
therefore looks like it forgot.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d6bb5b4307

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/Database.java
The SQLite half of this was fixed last commit; this is the network half,
and it is worse. Postgres and MySql each own a Wire, and a Wire owns ONE
16KB buffer with a position and a limit plus one output stream every
message is built in; MySql also carries the packet sequence number. Two
handlers sharing a Database therefore write into the same message buffer
and move each other's parse position -- protocol corruption, not merely
one request's rows committed by another's COMMIT.

Measured against a real PostgreSQL rather than argued from the source:

    with the lock       28s, every transaction wrote both its rows
    without the lock    661s, four threads dead, zero rows

The unsynchronized run does not fail fast, it WEDGES: pg_stat_activity
showed the connection still 'active' while the client waited for a reply
that the desynchronized stream would never produce, and the run only
ended when the test's own 60s joins expired. A hung connection per
request is the failure a reviewer would meet in production.

execute, query and transaction now hold the Database monitor. The SQLite
path then takes Db's monitor underneath -- always in that order, never
the reverse, so there is no cycle -- and both are reentrant, which is
what lets transaction() keep the session across the whole callback.

The check lives in DbCheck, which runs against every configured engine on
both runtimes, so CI covers Postgres and MySQL where it supplies them. It
drops its table before creating it: the finally cannot fire if the run is
killed, and the next run against a SHARED server then meets "relation
already exists" -- one interrupted run failing every run after it. That
happened here while A/B-ing this fix.

Unrelated to the fix: the self-test's zero-length-read probe connected
with a 2s timeout and failed once, beside the HTTP suite's four servers,
in a way I could not reproduce in three attempts. The timeout is 15s now.
Its outcome string already distinguishes "threw" from a wrong number, so
a real failure there will still say so rather than hiding behind the
timeout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74b5d60c3e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/StaticFiles.java
  * A static-file path spelled in valid hex that is not valid UTF-8 was
    decoded with U+FFFD substituted, so %C3%28 resolved to whatever a
    name genuinely containing U+FFFD resolves to -- while a bad hex DIGIT
    was already refused. One file, two spellings, one of them checked.
    Utf8.isValid was sitting in the same package; this decoder is the
    third copy of the rule and the last one that did not use it.

    Its parseInt(_, 16) took a sign as well, so "%+1" spelled the byte 1
    a third way. Two hex digits, tested as digits, like the generated
    decoders now do.

  * @ResponseStatus on a method that returns HttpServer.Response is
    refused. emitRoute returns the handler's Response untouched, so the
    annotation named a status that could never be sent. Refused rather
    than applied: overwriting the status of a Response the handler built
    would be the more surprising of the two, since it may already carry
    headers and a body chosen to match it.

  * required=false on a primitive query or header parameter is refused
    unless it has a defaultValue. The converter substitutes 0 or false,
    so the handler cannot tell an omitted value from a client that sent
    zero, while the annotation documents optional as null-bearing.

    The first version of that error told the developer to use a boxed
    type. Path, query and header parameters bind to String and the
    primitives only, so that advice was impossible to follow -- the test
    for the accept side is what caught it. It recommends a defaultValue
    now and says why a boxed type is not the way out here.

Each rule has a test for the shape it must NOT break: a Response return
without the annotation still sends the handler's own status, a defaulted
primitive and an optional String still compile, and caf%C3%A9.html still
reaches the lookup instead of being refused as malformed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7e74e25a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/demo/common/com/demo/GreeterService.java
Comment thread vm/backend/src/com/codename1/backend/Database.java Outdated
Comment thread vm/backend/src/com/codename1/backend/HttpServer.java Outdated
shai-almog added a commit that referenced this pull request Sep 10, 2026
The Linux screenshot suite does not deadlock. The event dispatch thread dies and
the process keeps running:

  [EDT] Exception: java.lang.ArrayIndexOutOfBoundsException - -1567458274
      at com_codename1_ui_Display.callSeriallyOnIdle:1174
      ...
  java.lang.NullPointerException
      at com_codename1_ui_util_EventDispatcher.fireActionEvent:305
      at com_codename1_ui_Display.mainEDTLoop:1611     <- inside the catch block
      at com_codename1_ui_RunnableWrapper.run:139
      at com_codename1_impl_CodenameOneThread.run:179
      at java_lang_Thread.runImpl:216                   <- top frame of the EDT

An exception reaches mainEDTLoop's catch. The catch reports it through the
application's error handler. The handler throws. That second throwable
propagates out of the catch, out of the dispatch loop and off the end of the
thread -- the trace reaches Thread.runImpl, so the thread is gone. Every other
thread lives, so the process stays up; nothing paints or handles input again.
The suite then sits idle until a 40-minute cap kills it with 13 of 100
screenshots never taken.

Four of the calls in that block run code Display does not own: a registered
CrashReport, the port's handleEDTException, the application's error handler, and
Dialog.show -- which paints, so it fails for any reason painting fails. Any one
of them ending the dispatch thread is the same permanent freeze, and this is
reachable from ordinary application code rather than being specific to CI.

There were TWO such blocks, not one: the dispatch loop's, and the phase that
runs before the first Form is shown. Both now go through reportEdtException,
which cannot let a second throwable escape. Both throwables are logged, the
original first so it is not buried by the failure to report it.

Unifying them means the pre-Form phase now also calls
CodenameOneThread.handleException, which it did not before. That is deliberate:
the method only acts when Log.isCrashBound(), so a crash-bound application was
silently losing exceptions from that phase.

EdtExceptionReportingTest covers one collaborator per test rather than one
representative case, because the hazard is per call site and wrapping three of
four would look fixed. Verified non-vacuous: with the guard removed all four
fail, each naming its own collaborator -- including the default path, where an
application that registers nothing still dies, there on
ExceptionInInitializerError out of Dialog.show.

What this does NOT fix is whatever produced the original garbage exception. That
is pre-existing (identical stack on PR #5741), x86_64+glibc only (arm64 and musl
pass in the same run from the same zig cc), and the exception TYPE varies
between occurrences at a fixed site, which is the signature of reading garbage
rather than a logic error; the stack is semantically impossible too, since
callSeriallyOnIdle is not called from paintDirty. With this change the EDT
survives it, so the next occurrence logs and the suite continues instead of
stalling -- which is what will make that corruption diagnosable.

Core suite: 6,617 tests, no failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f the log

  * A chunk size was read with Integer.parseInt(_, 16), which is not a hex
    parser for protocol input: it takes a leading SIGN, so "+1" framed a
    one-byte chunk and "-0" framed the TERMINATING one, and it takes any
    Unicode digit Character.digit knows, so U+0661 framed a chunk too. A
    conforming proxy in front rejects all three -- and a server that frames
    a message differently from the intermediary ahead of it is the whole of
    request smuggling, which is why this parser already refuses bare LF and
    obsolete folding. The trim() went with it: HTTP does not allow space
    around the size.

    This was the FOURTH time the same leniency produced a defect here,
    after percent escapes in static paths, in generated routers and in
    generated dispatchers. So it is one implementation now (Hex), not a
    fifth copy: the JSON \u escape used the same call and accepted
    "\u+041", and StaticFiles' private copy now defers to it too.

  * addPet ran the insert and lastInsertId as two synchronized calls
    rather than one operation. The connection is the same -- the comment
    there was already right about that -- but Db locks per call, so a
    second request could insert in the gap and hand the first response the
    other pet's id. It runs in a transaction now, which holds the
    connection across both.

  * A malformed port put the whole database URL, password included, into
    an IOException that goes to a log. describe() exists precisely to keep
    passwords out of strings like that; this error path bypassed it.

The chunk test pins the accept side too -- a plain "5" and a "5;ext=1"
extension both still frame a body -- because a size parser that refused
everything would pass a test written only against the bad spellings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Shai Almog <67850168+shai-almog@users.noreply.github.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e843735c33

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/DbPool.java
Comment thread vm/backend/src/com/codename1/backend/aws/S3.java Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e843735c33

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/ByteCodeTranslator/src/cn1_globals.m
Comment thread vm/backend/src/com/codename1/backend/DbPool.java
Comment thread vm/backend/native/cn1_backend_tlsclient.c
…e point

  * DbPool.open created a connection, configured it, and only then added it
    to the list close() walks. setBusyTimeout or enableWriteAheadLog can
    throw -- WAL is refused on some filesystems, which is the case someone
    meets in production and not in a test -- and the catch below calls
    pool.close(), which could only ever close what the list already held.
    That connection's native SQLite handle had no path to a close() at all,
    so every retry leaked another descriptor. Registered before configured.

  * The XML entity decoder cast the parsed value to char, so the legal
    &#x1F600; became U+F600: an object key came back different from the one
    stored, and a continuation token spelled that way pages from the wrong
    place -- a wrong ANSWER rather than an error. The numeric parse was also
    Integer.parseInt, which takes a sign and any Unicode digit, so "&#x+41;"
    and "&#+65;" were two more spellings of "A".

Two things worth recording, both mine.

The first version used StringBuilder.appendCodePoint, which vm/JavaAPI does
not define. It compiled for the Java SE arm against the real JDK and failed
the TRANSLATED build -- the rule that core code may only call what the VM
actually has. The surrogate pair is written out by hand now.

And that failure nearly passed as green. BackendTestSupport turns "could
not build the self-test binary" into an ASSUMPTION, so surefire reported a
skip, the other suite ran zero tests, and maven printed BUILD SUCCESS. The
compile error was only visible in the skip message inside the surefire XML.
A run that builds nothing and asserts nothing must not look like a pass;
checking that the test COUNT was non-zero is what caught it.

The S3 check added here says in its own comment what it does not prove:
MinIO returns the key as raw UTF-8 rather than as a numeric entity, so it
passes with the narrowing bug present. It is kept for the round trip it
does cover, and the narrowing fix rests on language semantics instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36428a5bb2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vm/backend/src/com/codename1/backend/Http1Date.java Outdated
Comment thread vm/backend/src/com/codename1/backend/Database.java Outdated
shai-almog and others added 2 commits September 11, 2026 12:02
Two places read a number with Integer.parseInt and then checked only its
upper bound, so a leading '-' passed a width test and reached code that
read the result as something else entirely.

Http1Date pins the date's SHAPE by fixed offsets, which constrains each
field's width but not its characters: "Sun, 06 Nov 9999 -1:-1:-1 GMT" is
exactly 29 characters with every separator in place, and each field read
as -1. The range test caught none of them, so a malformed conditional
request became a year-9999 timestamp -- which StaticFiles.isNotModified()
reads as newer than any file, answering 304 with no content to a client
that had nothing cached. Fields are now read by a digits() that accepts
only ASCII digits, which also refuses the '+1' and space-padded spellings
the old trim()-then-parse let through; IMF-fixdate is the one form this
parser claims to support and its fields are 2DIGIT and 4DIGIT.

Database.Url.parse had the same hole with worse consequences.
"postgres://u:p@host:-1/db" parsed cleanly, and both Postgres.connect and
MySql.connect read any non-positive port as "unset" and substitute their
default -- so the URL connected to 5432, silently, at the host that WAS
named. Measured before the fix: "Connection to 127.0.0.1:5432 failed" for
a URL that said port -1. An explicitly spelled port is now 1..65535 in
plain digits or the parse fails; an absent port still takes the default,
which is seeded before this branch runs.

Both are covered in SelfTest, and both were A/B'd by reverting the guard:
the date cases returned 253397458739000 and the port cases reached 5432.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BibopPageFloorIntegrationTest fails on the arm64 CI leg by giving back
NOTHING across its whole settle -- 240500KB of 266000KB still resident,
80 of 80 rounds spent -- and the memory then comes back during the next
phase. That shape is not a slow collector, and raising the budget did not
help when it was tried: it is a root the probe itself is holding.

CN1_CONSERVATIVE_GC_ROOTS is defined in every shipping build, so the
collector scans the native stack a word at a time and cannot tell a live
reference from a dead frame's leftover one. smallPhase built the ring in
its OWN frame, nulled the Java local, and then called releasePhase from
that same frame -- and scrubStack recurses DEEPER, so it overwrites the
region below the allocating frame and never the frame itself. Any copy
the C compiler spilled there outlives the settle, and one word is enough:
the 786k elements hang off the array, so the whole 192MB stands on a
single slot. The next phase's frames land on it, which is precisely when
the memory was observed to return. It is bimodal rather than slow because
whether a copy survives is a register allocation decision -- same source,
one arch keeping it and another not.

Each phase now builds and drops its live set in a method that has
RETURNED before the caller settles, which puts those words below the
stack pointer where scrubStack's own frames overwrite them.

Also aligned the probe's settle target with the fraction the harness
asserts on. They were 60% and 55%, so a settle could stop satisfied at
58% and the assertion fail on a release that was still arriving.

NOT REPRODUCED LOCALLY, and this cannot be: the probe passes on macOS and
on aarch64 Linux in a container, which is why the failure only ever
appears on the CI leg. The reasoning is structural rather than measured --
scrubStack demonstrably cannot reach its caller's caller's frame -- so the
arm64 leg is what confirms it.

Separately, recorded why the dead-thread demand latch cannot wake a
collector already inside LOCK.wait(): neither death path has a thread
that may enter a Java monitor. An OS thread reaches it on the DYING
thread, whose TLD is already queued for drain; a virtual thread reaches
it on the carrier, where a monitor enter would block that carrier and
every virtual thread it carries on a lock the collector holds. The
exposure is bounded in gcIdleWaitMillis instead, which refuses the long
idle while demand stands, and the case the finding names cannot reach
the long idle at all -- it needs a quiet heap, and retiring that many
virtual threads is not quiet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c18e9264e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

* worker forever, and the pool is bounded on purpose -- open as many silent
* connections as there are workers and the server stops answering anyone.
*/
private static final int SOCKET_TIMEOUT_MILLIS = envInt("CN1_HTTP_TIMEOUT_MS", 15000);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require a positive socket timeout

When CN1_HTTP_TIMEOUT_MS is zero or negative, the timeout no longer protects pooled workers: the Java SE Deadlines implementation treats non-positive values as an unbounded blocking read, and native zero disables SO_RCVTIMEO. A negative value is worse in the packaged runtime because setsockopt fails during acceptAll() before the descriptor is added to liveConnections; its catch then calls drop(fd), which returns without closing an untracked descriptor, leaking one socket per connection. Clamp this setting to a positive default as is already done for the minimum body rate.

Useful? React with 👍 / 👎.

Comment on lines +119 to +120
if(decoded.endsWith("/")) {
decoded = decoded + indexFile;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Redirect the exact static mount prefix

When this handler is mounted at /assets and receives /assets, stripping the prefix leaves an empty target; these lines turn it into /index.html before the directory-stat branch can issue its slash redirect. The index is consequently served as 200 at /assets, causing relative references such as style.css to resolve against /style.css rather than /assets/style.css. Treat the empty post-prefix path as the mounted directory and redirect to /assets/ before appending the index file.

Useful? React with 👍 / 👎.

The packaged arm caches one OpenSSL SSL_CTX per CA path, so a bundle is
parsed once rather than on every handshake. Keying that cache on the path
alone was wrong: a CA bundle is a mount, and mounts are rotated under a
stable name -- a Kubernetes secret or configmap, cert-manager, an RDS
bundle refresh. The path never changes, so a long-lived backend went on
trusting the roots it read at startup, and every connection failed the
moment the database or service presented a certificate signed by the new
one, with a process restart as the only remedy. The Java SE arm builds
its trust factory per upgrade and never had this, so the two arms
disagreed about a deployment that is meant to be routine.

Each cached context now carries the identity of the file it was built
from -- st_dev, st_ino, st_size and mtime to the nanosecond. Inode and
device catch the atomic-rename form (Kubernetes swaps a symlink, and stat
follows it), size and mtime catch a rewrite in place, and the nanoseconds
matter because a bundle rewritten within one second at the same size and
inode is otherwise indistinguishable.

Build first, swap second. Every failure path returns with the existing
entry untouched, so a rotation caught halfway -- the file replaced but not
yet readable, or briefly truncated -- keeps serving the old context
instead of emptying the slot and handing the next caller a null. The
stamp is written only for a context that loaded, so the rebuild is
retried until one does. Dropping the cache's reference is safe while
handshakes are in flight: SSL_new took its own, so an SSL still using the
old roots finishes on them.

SelfTest covers it on both arms by copying a real bundle, rotating it in
place to a PEM that cannot load, and rotating back. Rotating good to
BROKEN is what makes it decisive -- a failed build is not cached, so
good-to-good would pass either way. A/B'd: with the stamp check removed,
"a rotated CA bundle is re-read" reports verified instead of refused.

The harnesses derive the bundle path instead of only passing the variable
through, because a check that needs an environment variable nobody sets
never runs and reads as green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants